🔍 Rust PR Review

Summary: Looks good overall — the init implementation is clean and idiomatic; two issues worth fixing before merge.

Findings

🐛 Bugs / Logic Issues

• prompts/update-ado-agentic-workflow.md:43 — Stale reference to the removed command: "place them in the conventional order used by ado-aw create". The create subcommand is deleted in this very PR, so this line will confuse any AI agent following the prompt. Should be reworded to something like "place them in the conventional front matter order" (without attributing it to a removed command).

🔒 Security Concerns

• templates/init-agent.md (Setup section) — The binary download instructions lack checksum verification:

  curl -sL https://github.com/githubnext/ado-aw/releases/latest/download/ado-aw-linux-x64 -o /tmp/ado-aw && chmod +x /tmp/ado-aw

  The compiled ADO pipelines themselves do verify checksums (sha256sum -c checksums.txt --ignore-missing), making this an inconsistency. An AI agent following this template installs an unverified binary. Consider adding the same verification pattern used in generated pipelines, e.g.:

  curl -sL https://github.com/githubnext/ado-aw/releases/latest/download/checksums.txt -o /tmp/ado-aw-checksums.txt
  sha256sum -c /tmp/ado-aw-checksums.txt --ignore-missing

✅ What Looks Good

• src/init.rs is clean and idiomatic — async tokio::fs, proper anyhow::Result, all errors wrapped with .with_context(), and the --force guard is correct.
• The path.as_deref() call in main.rs to bridge Option<PathBuf> → Option<&Path> is the right pattern.
• The write-requiring safe-outputs list in prompts/update-ado-agentic-workflow.md matches WRITE_REQUIRING_SAFE_OUTPUTS in src/safeoutputs/mod.rs exactly — nice to see that kept in sync.
• AGENTS.md and CLI reference updated correctly to reflect the replaced command.

Generated by Rust PR Reviewer for issue #187 · ● 806.7K · ◷

🔍 Rust PR Review

Summary: Looks good — clean, idiomatic replacement. One security concern worth addressing before merge.

Findings

🔒 Security Concerns

• templates/init-agent.md lines 18–23 — The template instructs AI agents to download the ado-aw binary from /releases/latest/ without pinning a version or verifying a checksum:

  curl -sL https://github.com/githubnext/ado-aw/releases/latest/download/ado-aw-linux-x64 -o /tmp/ado-aw && chmod +x /tmp/ado-aw

  This is inconsistent with how the compiled pipelines themselves download binaries (in src/compile/common.rs), which use a pinned version and sha256sum -c checksums.txt --ignore-missing. The latest redirect is a TOCTOU window — if the release is rolled back or the binary is replaced between check and download, the agent silently installs the wrong version. Even though this is in a developer-local context rather than a pipeline runner, it's the first artefact a user gets from ado-aw init and establishes a precedent. Consider using the {{ compiler_version }} constant (already tracked in common.rs) to pin the URL, matching the pattern used in the generated pipelines.

⚠️ Suggestions

• src/init.rs — no tests — The command is simple enough that a single integration test (create in temp dir, verify file exists; re-run without --force returns error; re-run with --force overwrites) would give confidence and catch regressions. The rest of the test suite has good coverage; this is the only new code path with none.

• Cargo.toml — inquire crate orphan check — inquire is retained because configure still uses it (noted in the PR description). Worth a one-line comment in Cargo.toml next to the dependency so future maintainers don't remove it when they see create.rs is gone.

✅ What Looks Good

• src/init.rs is a textbook-clean 46-line implementation: anyhow::Result, .with_context() on every fallible op, tokio::fs for async I/O, include_str! for zero-cost embedding of the template — all following existing project conventions precisely.
• The Commands::Init clap variant follows the same derive-macro pattern as every other command, and the dispatch wiring in main.rs is correct.
• AGENTS.md and README are updated accurately to reflect the new UX.
• Deleting 1,136 lines of wizard code in favour of 46 lines of bootstrapping is the right call given the AI-first authoring model.

Generated by Rust PR Reviewer for issue #187 · ● 572.4K · ◷

🔍 Rust PR Review

Summary: Looks good overall — clean, well-scoped PR. One dead dependency and one leftover feature flag worth cleaning up.

Findings

⚠️ Suggestions

• Cargo.toml:29 — terminal_size = "0.4.3" is now a dead dependency. Its only consumer was create.rs (deleted in this PR). Should be removed.

• Cargo.toml:28 — The inquire editor feature is no longer needed. configure.rs only uses inquire::Password, not inquire::Editor (which the feature enables and was only used by the wizard). Consider downgrading to inquire = { version = "0.9.2" } (no features needed for Password prompts).

• tests/init_tests.rs:11,47,79 — Temp dirs aren't cleaned up on test panic (cleanup only runs if all assertions pass). Low priority for a CLI integration test, but worth knowing. Using a RAII guard (e.g., tempfile::TempDir) would make cleanup unconditional.

✅ What Looks Good

• init.rs is clean and minimal — correct use of anyhow::bail! + .with_context(), no unwrap() in production paths, async I/O done right.
• str::replace correctly substitutes all 3 {{ compiler_version }} occurrences in the template (Linux/macOS/Windows blocks) — no leftover placeholders in the emitted file.
• Tests cover the three meaningful cases (create, refuse-overwrite, force-overwrite), and the CARGO_BIN_EXE_ado-aw pattern is the right way to locate the integration test binary.
• inquire verified as still live in configure.rs — the comment added to Cargo.toml is accurate.

Generated by Rust PR Reviewer for issue #187 · ● 870.3K · ◷

similarly done - awaiting final rust reviewer comments.

🔍 Rust PR Review

Summary: Looks good — clean implementation with two minor issues worth fixing before merge.

Findings

🐛 Bugs / Logic Issues

• templates/init-agent.md (macOS section): The macOS install snippet uses sha256sum -c which doesn't exist on macOS — it ships shasum -a 256 instead. The Linux section works fine; the macOS section will error out for anyone following it literally.

  - cd /tmp && grep "ado-aw-darwin-x64" checksums.txt | sha256sum -c - && chmod +x /tmp/ado-aw
  + cd /tmp && grep "ado-aw-darwin-x64" checksums.txt | shasum -a 256 -c - && chmod +x /tmp/ado-aw

⚠️ Suggestions

• prompts/update-ado-agentic-workflow.md line ~998: The field-ordering guidance reads "place them in the conventional order used by ado-aw create" — that command was deleted in this PR. The reference is now a dangling pointer; replace with "the conventional order below" or point to AGENTS.md.

✅ What Looks Good

• src/init.rs is clean, idiomatic Rust: anyhow::Result, proper .with_context() chains, include_str! for compile-time embedding, and the single .replace() call correctly substitutes all three {{ compiler_version }} occurrences in the template.
• Tests cover the three key scenarios (create, refuse-without-force, force-overwrite) with good assertions including the version-substitution check.
• PathBuf usage for user-supplied --path is safe — output is always constrained to the fixed .github/agents/ado-aw.agent.md suffix.
• Removing the editor feature from inquire is correct; the wizard was its only consumer.
• Breaking change is clearly documented in both the PR description and AGENTS.md.

Generated by Rust PR Reviewer for issue #187 · ● 852.7K · ◷

🔍 Rust PR Review

Summary: Looks good — clean, minimal implementation with solid test coverage. One minor synchronous-in-async inconsistency and a few observations worth noting.

Findings

⚠️ Suggestions

• src/init.rs:16 — agent_path.exists() is a synchronous filesystem call inside an async function that otherwise uses only tokio::fs. This introduces a TOCTOU (time-of-check/time-of-use) race, though for a local CLI it's cosmetic. Prefer tokio::fs::try_exists(&agent_path).await? to be consistent with the async context and handle IO errors explicitly rather than treating a failed stat as "not exists."

• Cargo.toml — The editor feature of inquire was removed. The PR description says inquire is still needed for configure (password prompts only). Worth verifying that no inquire::Editor::* usage crept into configure.rs or elsewhere — the successful cargo build is reassuring but a comment alongside the dependency clarifies intent (the inline comment # Used by configure command (Password prompts) already does this well; just flag for reviewers to confirm).

• templates/init-agent.md — The embedded template references prompt URLs pinned to main (e.g. https://raw.githubusercontent.com/githubnext/ado-aw/main/prompts/create-ado-agentic-workflow.md). This is intentional per the PR description, but means a breaking prompt change on main would silently affect all deployed agent files regardless of compiler version. Consider noting this design trade-off in AGENTS.md or the template itself so future contributors don't accidentally break deployed agents with prompt changes.

✅ What Looks Good

• src/init.rs — Minimal and correct. include_str! ensures compile-time embedding (no runtime file-not-found failures). The three occurrences of {{ compiler_version }} in the template are all substituted by str::replace (replaces all occurrences). Error handling uses .with_context() throughout — no bare unwrap()s.

• tests/init_tests.rs — Good integration test coverage: happy path, overwrite guard, and --force semantics. Verifying that {{ compiler_version }} is absent in output is exactly the right assertion.

• Cargo.toml — Correct removal of terminal_size (only used by the deleted wizard). Removing the editor feature from inquire is a valid cleanup.

• src/main.rs — Command removal and addition are clean and symmetric. The command_name match arm is updated consistently.

Generated by Rust PR Reviewer for issue #187 · ● 616.3K · ◷